Skip to content

[ISSUE #10375] Fix race condition between deleteTopic and FlushConsumeQueueService by removing getLifeCycle indirection#10376

Merged
lizhimins merged 1 commit into
apache:developfrom
f1amingo:fix/remove-getLifeCycle-indirection
May 25, 2026
Merged

[ISSUE #10375] Fix race condition between deleteTopic and FlushConsumeQueueService by removing getLifeCycle indirection#10376
lizhimins merged 1 commit into
apache:developfrom
f1amingo:fix/remove-getLifeCycle-indirection

Conversation

@f1amingo
Copy link
Copy Markdown
Contributor

Which Issue(s) This PR Fixes

Brief Description

This PR fixes a race condition in ConsumeQueueStore where a deleted topic could be unintentionally recreated in consumeQueueTable.

Root Cause: Methods like flush, recover, deleteExpiredFile, etc. delegated to getLifeCycle(topic, queueId) which internally called findOrCreateConsumeQueue. When FlushConsumeQueueService iterated consumeQueueTable and flushed a ConsumeQueue whose topic had just been removed by deleteTopic on another thread, findOrCreateConsumeQueue would recreate the deleted topic entry.

Fix: Remove the getLifeCycle indirection and invoke ConsumeQueueInterface methods directly on the already-held instance, eliminating any possibility of recreating a deleted topic. The old delegate methods are preserved with @deprecated for binary compatibility.

How Did You Test This Change?

Unit Test

@f1amingo f1amingo force-pushed the fix/remove-getLifeCycle-indirection branch from 98772b0 to c72194d Compare May 25, 2026 09:30
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 25, 2026

Codecov Report

❌ Patch coverage is 26.08696% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 48.79%. Comparing base (980f3d7) to head (ec92ad5).

Files with missing lines Patch % Lines
...apache/rocketmq/store/queue/ConsumeQueueStore.java 26.08% 16 Missing and 1 partial ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##             develop   #10376      +/-   ##
=============================================
- Coverage      48.90%   48.79%   -0.12%     
+ Complexity     13460    13422      -38     
=============================================
  Files           1376     1376              
  Lines         100539   100527      -12     
  Branches       12983    12983              
=============================================
- Hits           49169    49051     -118     
- Misses         45370    45466      +96     
- Partials        6000     6010      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@f1amingo f1amingo force-pushed the fix/remove-getLifeCycle-indirection branch from c72194d to 74ad874 Compare May 25, 2026 11:17
…eueService by removing getLifeCycle indirection

- Remove getLifeCycle() which delegates to findOrCreateConsumeQueue(), eliminating the risk of recreating a deleted topic's ConsumeQueue during concurrent flush operations
- Call ConsumeQueueInterface methods directly on the instance instead of re-looking up through findOrCreateConsumeQueue
- Deprecate delegate methods (load, recover, checkSelf, flush, deleteExpiredFile, truncateDirtyLogicFiles, swapMap, cleanSwappedMap, isFirstFileAvailable, isFirstFileExist) that previously used getLifeCycle indirection
- Remove unnecessary RocksDBException from getDispatchFromPhyOffset signature
- Fix maps.values().size() to maps.size() in recoverConcurrently
@f1amingo f1amingo force-pushed the fix/remove-getLifeCycle-indirection branch from 74ad874 to ec92ad5 Compare May 25, 2026 11:22
Copy link
Copy Markdown
Contributor

@imzs imzs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make sense for file CQ.

@lizhimins lizhimins merged commit 9e2d877 into apache:develop May 25, 2026
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Race condition between deleteTopic and FlushConsumeQueueService causes deleted topic to be recreated in consumeQueueTable

4 participants